Skip to content

Fix ty possibly-missing-attribute warnings in tests#267

Merged
daaain merged 1 commit into
mainfrom
fix/ty-test-warnings
Jul 7, 2026
Merged

Fix ty possibly-missing-attribute warnings in tests#267
daaain merged 1 commit into
mainfrom
fix/ty-test-warnings

Conversation

@daaain

@daaain daaain commented Jul 7, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

ctx.messages is list[Optional[TemplateMessage]] (ghost slots are None), so comprehensions iterating it directly need an 'm is not None' guard. In test_skill_pairing, collect the narrowed SessionHeaderMessage contents instead of the wrapper messages so the isinstance narrowing survives into the follow-up comprehensions.

Summary by CodeRabbit

  • Tests
    • Hardened several test assertions to safely ignore None entries while scanning message lists.
    • Improved message lookups for session headers, UUID ordering, session filtering, and rendered output checks.
    • Simplified one branch-header assertion by reading header details directly from the surviving session header objects.
    • No user-facing behavior or public APIs changed.

@daaain @claude
Fix ty possibly-missing-attribute warnings in tests
e71ccd3 
ctx.messages is list[Optional[TemplateMessage]] (ghost slots are None),
so comprehensions iterating it directly need an 'm is not None' guard.
In test_skill_pairing, collect the narrowed SessionHeaderMessage
contents instead of the wrapper messages so the isinstance narrowing
survives into the follow-up comprehensions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 11225553-fe8a-4a95-a80a-166ceba0c344

📥 Commits

Reviewing files that changed from the base of the PR and between 42e54d4 and e71ccd3.

📒 Files selected for processing (3)
  • test/test_dag_integration.py
  • test/test_hook_attachment_rendering.py
  • test/test_skill_pairing.py
📝 Walkthrough

Walkthrough

Test files updated to add explicit None guards in list/dict comprehensions scanning ctx.messages, preventing unguarded attribute access on None entries. One test also refactors branch-header lookup to extract SessionHeaderMessage objects directly instead of the wrapper message. No production code changes.

Changes

None-safe test message filtering

Layer / File(s) Summary
DAG integration comprehension guards
test/test_dag_integration.py		 Adds m is not None guards before isinstance/attribute access when building header_session_ids, msg_uuids, s2_msgs, and rendered_uuids.
Hook attachment rendering comprehension guards
test/test_hook_attachment_rendering.py		 Adds m is not None guards before isinstance checks for HookAttachmentMessage and SystemMessage in three test cases.
Skill pairing branch header extraction
test/test_skill_pairing.py		 Refactors branch_headers/skill_branches to collect SessionHeaderMessage objects directly and read parent_message_index from them.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • daaain/claude-code-log#131: Prior changes to the same test_skill_pairing.py assertions around SessionHeaderMessage and parent_message_index extraction.
  • daaain/claude-code-log#193: Introduced None ghost slots in ctx.messages, motivating the None-guard filtering added in these test comprehensions.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the changeset: it describes fixing ty warnings in tests, which is the main purpose of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ty-test-warnings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@daaain daaain merged commit 5817a39 into main Jul 7, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@daaain